feat(MessageComposer): introduce context for custom composers - #3249
feat(MessageComposer): introduce context for custom composers#3249arnautov-anton wants to merge 4 commits into
Conversation
58afcb4 to
def2530
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe message composer adds controller context access and configurable unmount cleanup. The Vite example adds inline message editing with a context-menu action, inline composer, cancel behavior, component registration, and styles. ChangesMessage composer and inline editing
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant User
participant MessageActions
participant InlineEditableMessage
participant MessageComposer
User->>MessageActions: Select "Edit inline"
MessageActions->>InlineEditableMessage: Start editing
InlineEditableMessage->>MessageComposer: Create and render composer
User->>MessageComposer: Edit or cancel message
MessageComposer->>InlineEditableMessage: End editing
InlineEditableMessage->>User: Render normal message UI
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Size Change: +5.68 kB (+0.64%) Total Size: 890 kB 📦 View Changed
ℹ️ View Unchanged
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3249 +/- ##
==========================================
- Coverage 85.18% 85.18% -0.01%
==========================================
Files 508 508
Lines 15966 15977 +11
Branches 5029 5034 +5
==========================================
+ Hits 13601 13610 +9
- Misses 2365 2367 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/MessageComposer/MessageComposer.tsx`:
- Around line 82-85: Update the affected MessageComposer props guide to document
the public preventClearingOnUnmount prop, including that setting it to true
prevents clearing the established MessageComposerController state when the
component unmounts. Keep the existing inline API documentation unchanged.
- Line 102: Update the guard in the MessageComposer flow before accessing
messageComposer.channel.disconnected to also handle a missing channel,
preserving the existing early return for disconnected channels and preventing
dereferencing an absent channel.
- Around line 108-112: Update the effect cleanup around messageComposer so
changes to props.preventClearingOnUnmount do not trigger cleanup while the
component remains mounted. Store the latest prop value in a ref and read that
ref inside the cleanup, while limiting the effect dependency array to
messageComposer; preserve the existing unmount guard and promise.finally clear
behavior.
- Around line 104-110: Update the cleanup flow around
messageComposer.createDraft() so draft-creation rejections are explicitly
handled rather than becoming unhandled promises. Preserve the
preventClearingOnUnmount early return while attaching rejection handling to the
started promise, and ensure the promise returned by finally() in the normal
clearing path is also consumed or otherwise handled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4aa86f9b-989b-450d-8875-a21196c7a8a5
📒 Files selected for processing (1)
src/components/MessageComposer/MessageComposer.tsx
| /** | ||
| * When set to `true` disables clearing established state of the MessageComposerController upon component unmount. | ||
| */ | ||
| preventClearingOnUnmount?: boolean; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Document the new public prop in the guide page.
This changes the public MessageComposerProps API. Add preventClearingOnUnmount and its unmount semantics to the affected MessageComposer props guide; the inline comment alone is insufficient.
As per coding guidelines, public API changes must update inline docs and affected guide pages.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/MessageComposer/MessageComposer.tsx` around lines 82 - 85,
Update the affected MessageComposer props guide to document the public
preventClearingOnUnmount prop, including that setting it to true prevents
clearing the established MessageComposerController state when the component
unmounts. Keep the existing inline API documentation unchanged.
Source: Coding guidelines
| if (props.preventClearingOnUnmount) return; | ||
|
|
||
| promise.finally(() => messageComposer.clear()); | ||
| }, | ||
| [messageComposer], | ||
| [messageComposer, props.preventClearingOnUnmount], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate MessageComposer files"
fd -a 'MessageComposer\.tsx$' . || true
echo
echo "Relevant snippet and nearby effect code"
if [ -f src/components/MessageComposer/MessageComposer.tsx ]; then
wc -l src/components/MessageComposer/MessageComposer.tsx
sed -n '1,170p' src/components/MessageComposer/MessageComposer.tsx | cat -n
fiRepository: GetStream/stream-chat-react
Length of output: 8005
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect controller draft/create/clear definitions"
rg -n "createDraft|clear\\(|drafts|state" src/components/MessageComposer -S
echo
echo "Inspect controller hooks"
fd -a 'Hooks?|Controller|.*Controller.*|.*Controller.*' src/components/MessageComposer | sed 's#^`#/`#' | head -50
echo
fd -a 'Hooks?|.*Controller.*' src/components/MessageComposer | while read -r f; do
echo "--- $f"
wc -l "$f"
sed -n '1,260p' "$f" | cat -n
done
echo
echo "Check package React version type/imports are safe for useRef"
if [ -f package.json ]; then
node -e "const p=require('./package.json'); console.log(JSON.stringify({react:p.dependencies?.react ?? p.devDependencies?.react}, null, 2))"
fi
rg "react|react-dom" package.json yarn.lock 2>/dev/null | head -40 || trueRepository: GetStream/stream-chat-react
Length of output: 29499
Avoid running unmount cleanup while re-running this effect.
props.preventClearingOnUnmount is a dependency, so changing it from false to true runs the existing cleanup before the new effect. That cleanup can call createDraft() and then clear() while the component is still mounted, dropping the current composer state. Key this only on messageComposer and read the latest prop from a ref.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/MessageComposer/MessageComposer.tsx` around lines 108 - 112,
Update the effect cleanup around messageComposer so changes to
props.preventClearingOnUnmount do not trigger cleanup while the component
remains mounted. Store the latest prop value in a ref and read that ref inside
the cleanup, while limiting the effect dependency array to messageComposer;
preserve the existing unmount guard and promise.finally clear behavior.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/vite/src/index.scss`:
- Line 12: Update the SCSS import in the stylesheet to use the configured direct
string import notation by removing the url() wrapper, while preserving the
existing path and layer(stream-app-overrides) declaration.
In `@examples/vite/src/InlineEditMessage/InlineEditMessage.tsx`:
- Around line 120-129: Update the editingComposer useMemo dependency array to
use channel.cid instead of channel, while retaining compositionContext: channel
in the MessageComposerController configuration and leaving the other
dependencies unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 39ab8a54-4389-4d1f-8724-3a0d9ed00568
📒 Files selected for processing (5)
examples/vite/src/App.tsxexamples/vite/src/InlineEditMessage/InlineEditMessage.scssexamples/vite/src/InlineEditMessage/InlineEditMessage.tsxexamples/vite/src/InlineEditMessage/index.tsexamples/vite/src/index.scss
| @import url('./AppSettings/AppSettings.scss') layer(stream-app-overrides); | ||
| @import url('./CustomMessageActions/CustomMessageActions.scss') | ||
| layer(stream-app-overrides); | ||
| @import url('./InlineEditMessage/InlineEditMessage.scss') layer(stream-app-overrides); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the configured SCSS import notation.
Line 12 violates the import-notation rule. This adds a Stylelint error. Remove the url() wrapper.
Proposed fix
-@import url('./InlineEditMessage/InlineEditMessage.scss') layer(stream-app-overrides);
+@import './InlineEditMessage/InlineEditMessage.scss' layer(stream-app-overrides);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @import url('./InlineEditMessage/InlineEditMessage.scss') layer(stream-app-overrides); | |
| `@import` './InlineEditMessage/InlineEditMessage.scss' layer(stream-app-overrides); |
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 12-12: Expected "url('./InlineEditMessage/InlineEditMessage.scss')" to be "'./InlineEditMessage/InlineEditMessage.scss'" (import-notation)
(import-notation)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/vite/src/index.scss` at line 12, Update the SCSS import in the
stylesheet to use the configured direct string import notation by removing the
url() wrapper, while preserving the existing path and
layer(stream-app-overrides) declaration.
Source: Linters/SAST tools
| const editingComposer = useMemo(() => { | ||
| if (!isEditing) return undefined; | ||
|
|
||
| return new MessageComposerController({ | ||
| client, | ||
| composition: message, | ||
| compositionContext: channel, | ||
| config: { drafts: { enabled: false } }, | ||
| }); | ||
| }, [channel, client, isEditing, message]); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the target file and relevant inline edit implementation.
if [ -f "examples/vite/src/InlineEditMessage/InlineEditMessage.tsx" ]; then
nl -ba examples/vite/src/InlineEditMessage/InlineEditMessage.tsx | sed -n '1,180p'
else
echo "Target file not found"
fd -i 'InlineEditMessage.tsx' .
fi
# Show how MessageComposerController/types and channel dependencies are used elsewhere, if available.
rg -n "MessageComposerController|compositionContext: channel|const editingComposer|useMemo|channel\\b" examples/vite/src || true
# Deterministic static check: parse the target JSX and report all useMemo/compose dependency expressions containing identifier `channel` in the target file.
python3 - <<'PY'
from pathlib import Path
p=Path('examples/vite/src/InlineEditMessage/InlineEditMessage.tsx')
if not p.exists():
print("MISSING_PATH")
raise SystemExit(0)
text=p.read_text()
# Simple brace-aware extraction around useMemo(...) calls for the exact controller construction area.
hits=[]
for i,line in enumerate(text.splitlines(),1):
if 'new MessageComposerController({' in line or 'editingComposer' in line or "compositionContext: channel" in line:
hits.append((i,line.strip()))
print("target_hits:")
for h in hits:
print(f"{h[0]}:{h[1]}")
# Locate the useMemo dependency list for editingComposer by finding bracket balance from first useMemo opening paren until matching closed paren.
lines=text.splitlines()
start=None
for idx,line in enumerate(lines):
if 'const editingComposer = useMemo' in line or 'useMemo(() => {' in line and 'compositionContext: channel' in '\n'.join(lines[idx:idx+10]):
start=idx
break
if start is None:
print("NO_EDITING_COMPOSER_MEMO")
raise SystemExit(0)
buf=[]
state=0
for idx,line in enumerate(lines[start:], start+1):
for ch in line:
if ch == '(':
state += 1
if state == 1:
found=opening=(idx, line.find(ch)+1)
if ch == ')':
state -= 1
if state == 0:
end=(idx, line.find(ch)+1)
break
if state == 0 and opening:
break
snippet='\n'.join(lines[opening[0]-1:end[0]-1])
print("editingComposer_memosnippet:")
print(snippet)
print("contains_identifier_channel:", ' channel' in snippet or ',channel' in snippet or '(channel' in snippet or '] [channel' in snippet or '] channel' in snippet)
PYRepository: GetStream/stream-chat-react
Length of output: 205
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | grep -F 'examples/vite/src/InlineEditMessage/InlineEditMessage.tsx' || true
echo
echo "Target file lines:"
awk '{printf "%6d\t%s\n", NR, $0}' examples/vite/src/InlineEditMessage/InlineEditMessage.tsx | sed -n '1,180p'
echo
echo "Relevant searches:"
rg -n "MessageComposerController|compositionContext: channel|const editingComposer|useMemo|channel\\b" examples/vite/src/InlineEditMessage/InlineEditMessage.tsx || true
echo
echo "Static dependency extraction:"
python3 - <<'PY'
from pathlib import Path
p=Path('examples/vite/src/InlineEditMessage/InlineEditMessage.tsx')
text=p.read_text()
lines=text.splitlines()
start=None
for idx,line in enumerate(lines):
if 'const editingComposer = useMemo' in line:
start=idx
break
print("start=", start)
if start is None:
raise SystemExit(0)
buf=[]
state=0
opening=None
for idx=line_idx in range(start, len(lines)):
line=lines[idx]
if opening is None and '(' in line:
opening=(idx, line.find('(')+1)
state += 1
for ch in line[line.find('(')+1:]:
if ch =='(': state +=1
if ch ==')': state -=1
elif opening is not None:
for ch in line:
if ch == '(': state += 1
if ch == ')':
state -= 1
if state == 0:
end=(idx, line.find(ch)+1)
break
if state == 0 and end:
break
else:
end=None
snippet='\n'.join(lines[opening[0]-1:end[0]-1])
print("snippet:")
print(snippet)
print("contains_identifier_channel:", ' channel' in snippet or ',channel' in snippet or '[channel' in snippet or '](channel' in snippet)
PYRepository: GetStream/stream-chat-react
Length of output: 7402
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Current dependency array and context use:"
awk '{printf "%6d\t%s\n", NR, $0}' examples/vite/src/InlineEditMessage/InlineEditMessage.tsx | sed -n '112,129p'
echo
echo "Search for channel.*cid in examples:"
rg -n "channel\s*:|channel\.cid|useChannelStateContext|MessageComposerState|MessageComposerProvider" examples/vite/src -g '*.ts' -g '*.tsx' | sed -n '1,120p' || trueRepository: GetStream/stream-chat-react
Length of output: 5785
Use channel.cid in the useMemo dependency list.
The unstable channel object is in the dependency array, and the project guideline for **/*.{ts,tsx} requires channel.cid in dependency arrays. Replace channel with channel.cid while keeping compositionContext: channel for the composer context.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/vite/src/InlineEditMessage/InlineEditMessage.tsx` around lines 120 -
129, Update the editingComposer useMemo dependency array to use channel.cid
instead of channel, while retaining compositionContext: channel in the
MessageComposerController configuration and leaving the other dependencies
unchanged.
Source: Coding guidelines
662be9e to
206eb07
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
206eb07 to
dea6846
Compare
dea6846 to
a481734
Compare
a481734 to
da7170b
Compare
🎯 Goal
Fixes: #3248
Closes: REACT-1046
As a side feauture, adds
preventClearingOnUnmountprop.Summary by CodeRabbit
New Features
Bug Fixes